Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update Brave Stat text on New Tab Page from "Ads and Trackers blocked" to "Trackers & ads blocked" #5630

Merged
merged 2 commits into from
Jun 1, 2020
Merged

Update Brave Stat text on New Tab Page from "Ads and Trackers blocked" to "Trackers & ads blocked" #5630

merged 2 commits into from
Jun 1, 2020

Conversation

0xedward
Copy link
Contributor

@0xedward 0xedward commented May 22, 2020 •
edited by bsclifton
Loading

Resolves brave/brave-browser#9692
Resolves brave/brave-browser#9838

Submitter Checklist:

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

image

@0xedward 0xedward force-pushed the change-brave-stats-text-ntp branch from ebe47ea to ce30c61 Compare May 27, 2020 19:08
@rebron rebron requested review from AndriusA and bsclifton May 29, 2020 20:11
@rebron rebron mentioned this pull request May 29, 2020
Closed
@bsclifton bsclifton force-pushed the change-brave-stats-text-ntp branch from ce30c61 to dd88967 Compare May 29, 2020 21:27
@bsclifton
Copy link
Member

LGTM! @0xedward would you be able to take on brave/brave-browser#9838 in this same pull request? 😄

@0xedward
Copy link
Contributor Author

@bsclifton Sure thing! I'm working on it now

@0xedward
Update bandwidth and time saved text on NTP
12a9ceb 
Change "Estimated bandwidth saved" to "Bandwidth saved"
Change "Estimated time saved" to "Time saved"

Fix brave/brave-browser#9838
@bsclifton bsclifton requested a review from mkarolin June 1, 2020 05:30
@bsclifton bsclifton added this to the 1.11.x - Nightly milestone Jun 1, 2020
bsclifton
bsclifton approved these changes Jun 1, 2020
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiled on Windows; changes LGTM!
image

@mkarolin can you check out also just to make sure I didn't miss something? Change shouldn't match anything we're manually doing search/replaces for (ex: Google => Brave) so I think we're good to go. Could be great to accept before we do Chromium 83 translations 😄

@bsclifton bsclifton added the CI/skip Do not run CI builds (except noplatform) label Jun 1, 2020
@bsclifton
Copy link
Member

Marked as CI/skip since CI doesn't run for branches in forks - but I did verify this builds locally and all tests pass (I ran unit tests and browser tests on Windows)

mkarolin
mkarolin approved these changes Jun 1, 2020
View reviewed changes
Copy link
Collaborator

@mkarolin mkarolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @bsclifton I don't think this file goes through the branding replacement script since this one is fully controlled by us.

@bsclifton bsclifton merged commit aad2ed5 into brave:master Jun 1, 2020
@bsclifton
Copy link
Member

Thanks for the contribution, @0xedward 😄! I'll try and circle back on your other ones now

@rebron
Copy link
Collaborator

rebron commented Jun 1, 2020

Thanks @0xedward Excellent!

@mkarolin @bsclifton Just to confirm that we do want this for 1.10.x as the stat will be updated then.

mkarolin pushed a commit that referenced this pull request Jun 2, 2020
@bsclifton @mkarolin
Merge pull request #5630 from 0xedward/change-brave-stats-text-ntp
e9e4a0a 
Update Brave Stat text on New Tab Page from "Ads and Trackers blocked" to "Trackers & ads blocked"
@mkarolin mkarolin mentioned this pull request Jun 2, 2020
Merged
32 tasks
@0xedward 0xedward deleted the change-brave-stats-text-ntp branch October 2, 2020 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsclifton bsclifton bsclifton approved these changes

@mkarolin mkarolin mkarolin approved these changes

@AndriusA AndriusA Awaiting requested review from AndriusA

Assignees

@0xedward 0xedward

Labels
CI/skip Do not run CI builds (except noplatform)
Projects
None yet
Milestone
1.11.x - Release #1
4 participants
@0xedward @bsclifton @rebron @mkarolin